Make a deep copy of cluster prior to calling actuator #426
Conversation
k8s-ci-robot
added
cncf-cla: yes
|
/assign @mkjelland |
|
@mkjelland PTAL, this PR only modifies the handling of the informer-provided object provided to the Cluster Actuator. |
pkg/controller/cluster/controller.go
Outdated
| name := cluster.Name | ||
| // Deep-copy otherwise we are mutating our cache. | ||
| clusterCopy := cluster.DeepCopy() | ||
| name := clusterCopy.Name | ||
| glog.Infof("Running reconcile Cluster for %s\n", name) | ||
|
|
||
| if !cluster.ObjectMeta.DeletionTimestamp.IsZero() { |
There was a problem hiding this comment.
Looking at the machine controller, it seems like this line and line 81 should also use clusterCopy instead of cluster. Is there a reason to use the original object in these cases?
There was a problem hiding this comment.
There was not, fixed in latest. Thanks!
|
/test pull-cluster-api-test |
|
lgtm @karan @rsdcastro: Can one of you add the lgtm label? |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: detiber, roberthbailey The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
Add hack/release.sh script
What this PR does / why we need it:
Currently the Machine Controller and Custer Controller differ in the handling of the informer provided object that they pass into the Actuators. This PR aligns the Cluster Controller handling of the informer provided object to match that of the Machine Controller.
Changes:
Which issue(s) this PR fixes:
Fixes #420
Release note:
@kubernetes/kube-deploy-reviewers